Skip to content

feat(diagnostics): add opt-in span context to error logs - #111

Merged
danielkov merged 7 commits into
mainfrom
fix/issue-48-effect-provenance
Sep 5, 2026
Merged

feat(diagnostics): add opt-in span context to error logs#111
danielkov merged 7 commits into
mainfrom
fix/issue-48-effect-provenance

Conversation

@danielkov

@danielkov danielkov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Add opt-in, bounded local tracing span context alongside existing fatal error logs, independently of OpenTelemetry export. capture_error_spans defaults to false; enabling it attaches an optional span_context field to schema-v2 records.

Impact

Disabled collection installs no diagnostic layer or retained span history. Built-in TUI and acp.kit children inherit the resolved setting and require compatible Kit executables. Existing errors, cancellation handling, and diagnostic-file compatibility are preserved.

Technical details

Bounded local span history

Operation spans remain alive through execution and existing error-log writes. Span extensions retain approved structured fields from active and already-closed children, capped at 24 fragments, eight descendant levels, six fields per fragment, 32-byte string values, and 12 KiB of serialized context. Arbitrary identifiers, Debug/Display values, messages, prompts, tool arguments/results, URLs, and provider payloads are excluded even when OTEL message capture is enabled.

Diagnostic scope

Snapshots are partial operation histories, not causal error chains, effect guarantees, or replay-safety evidence. Upstream TaskManager execution spawns and Compose blocking/worker boundaries do not fully propagate tracing ancestry; remote child spans are not transported into parent logs. Collection and persistence remain best-effort at existing error boundaries.

Hidden-tool wrappers also preserve native interruption and pre-invocation failure outcomes rather than collapsing them through the legacy invocation API.

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abandoned fork calls can discard captured possible-effects observations before they reach the local fatal diagnostic.

Comment thread src/tools/subagent.rs Outdated
@danielkov danielkov changed the title fix(subagents): retain possible-effects observations on failure fix(runtime): retain possible-effects observations on failure Sep 5, 2026
Comment thread src/tools/subagent.rs Outdated
Comment thread src/fatal.rs Outdated
@danielkov

Copy link
Copy Markdown
Contributor Author

Addressed in a61fa9f.

docs/issues/possible-effects.md was an implementation/coordination diary mistakenly committed as product documentation. Removed it, rather than replacing it with another process document; the existing session docs now describe only the diagnostic behavior.

The 2 → 3 → 4 markers tracked two unmerged implementation phases, not a necessary compatibility boundary with the shipped format. Restored writer schema 2: the shipped v2 top-level reader ignores the additive possible_effects field, including local_session. A frozen v2 reader now verifies preservation of all known fields from actual extended writer output. New readers default missing effects to unknown/incomplete, preserve supplied effects independently of the numeric marker, and still reject malformed sources, types, extra fields, and completeness claims. Existing branch-produced 3/4 records remain naturally readable without rewrites.

Both argument-count suppressions were replaced with named parameter types. This remains a draft preparatory PR; full typed producer/transport integration is still release-gated, so this does not close #48.

@danielkov

Copy link
Copy Markdown
Contributor Author

Follow-up: 04c7335 also removes the remaining PR-added argument-count suppression in ACP v2 by grouping lifecycle/effects instruments in SessionInstruments. No new Clippy suppressions remain in the PR; unrelated existing exceptions were left alone.

The entire PR and its follow-up changes have an independent review with no actionable findings at this exact head, and both current-head CI jobs pass. Both user threads are addressed and resolved. The PR remains draft because full typed producer/transport integration is still release-gated.

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A2A executor timeout bypasses persistence of captured observations. ACP finalization writes diagnostics but omits their paths from terminal errors.

Comment thread src/protocols/a2a.rs Outdated
Comment thread src/protocols/acp/v2.rs Outdated
@danielkov danielkov changed the title fix(runtime): retain possible-effects observations on failure feat(diagnostics): add opt-in span context to error logs Sep 5, 2026
@danielkov

Copy link
Copy Markdown
Contributor Author

Reworked in b650191. capture_error_spans defaults to false and independently enables bounded, privacy-filtered tracing context beside existing schema-v2 fatal errors; OTLP is not required. The effects accumulators, transport plumbing, and effects-only cancellation/finalization writes are removed.

Independent full-diff and exact-head reviews are clean. Local strict validation and both configured CI jobs passed at this head: https://github.com/speakeasy-api/kit/actions/runs/33993518142.

Keeping this draft diagnostic-only. A real Agent→AsyncTaskManager→Tool regression confirms missing invocation ancestry even for foreground tools; Compose/Runlet thread boundaries and remote child spans also remain incomplete. These partial histories do not establish replay safety or fulfill all original #47/#48 acceptance; recovery remains outside this PR.

kit-code-agent[bot]
kit-code-agent Bot previously approved these changes Sep 5, 2026

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. The changes look good to merge.

@danielkov
danielkov marked this pull request as ready for review September 5, 2026 22:17
@danielkov
danielkov enabled auto-merge (squash) September 5, 2026 22:17

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. The changes look good to merge.

@danielkov
danielkov merged commit 7026c4a into main Sep 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant